From 3181ef24f9c7550117290eedb1be11196c2353a1 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 24 Aug 2017 12:54:57 -0700 Subject: [PATCH] Add support for nightly features in Cargo --- src/cargo/core/features.rs | 211 +++++++++++++++++++++++++++++++++ src/cargo/core/manifest.rs | 24 +++- src/cargo/core/mod.rs | 4 +- src/cargo/lib.rs | 1 - src/cargo/ops/cargo_package.rs | 4 + src/cargo/util/toml/mod.rs | 13 +- tests/cargo-features.rs | 154 ++++++++++++++++++++++++ tests/cargotest/lib.rs | 16 +++ 8 files changed, 423 insertions(+), 4 deletions(-) create mode 100644 src/cargo/core/features.rs create mode 100644 tests/cargo-features.rs diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs new file mode 100644 index 000000000..4a24024c1 --- /dev/null +++ b/src/cargo/core/features.rs @@ -0,0 +1,211 @@ +//! Support for nightly features in Cargo itself +//! +//! This file is the version of `feature_gate.rs` in upstream Rust for Cargo +//! itself and is intended to be the avenue for which new features in Cargo are +//! gated by default and then eventually stabilized. All known stable and +//! unstable features are tracked in this file. +//! +//! If you're reading this then you're likely interested in adding a feature to +//! Cargo, and the good news is that it shouldn't be too hard! To do this you'll +//! want to follow these steps: +//! +//! 1. Add your feature. Do this by searching for "look here" in this file and +//! expanding the macro invocation that lists all features with your new +//! feature. +//! +//! 2. Find the appropriate place to place the feature gate in Cargo itself. If +//! you're extending the manifest format you'll likely just want to modify +//! the `Manifest::feature_gate` function, but otherwise you may wish to +//! place the feature gate elsewhere in Cargo. +//! +//! 3. Do actually perform the feature gate, you'll want to have code that looks +//! like: +//! +//! ```rust,ignore +//! use core::{Feature, Features}; +//! +//! let feature = Feature::launch_into_space(); +//! package.manifest().features().require(feature).chain_err(|| { +//! "launching Cargo into space right now is unstable and may result in \ +//! unintended damage to your codebase, use with caution" +//! })?; +//! ``` +//! +//! Notably you'll notice the `require` funciton called with your `Feature`, and +//! then you use `chain_err` to tack on more context for why the feature was +//! required when the feature isn't activated. +//! +//! And hopefully that's it! Bear with us though that this is, at the time of +//! this writing, a very new feature in Cargo. If the process differs from this +//! we'll be sure to update this documentation! + +use std::env; + +use util::errors::CargoResult; + +enum Status { + Stable, + Unstable, +} + +macro_rules! features { + ( + pub struct Features { + $([$stab:ident] $feature:ident: bool,)* + } + ) => ( + #[derive(Default, Clone, Debug)] + pub struct Features { + $($feature: bool,)* + activated: Vec, + } + + impl Feature { + $( + pub fn $feature() -> &'static Feature { + fn get(features: &Features) -> &bool { + &features.$feature + } + static FEAT: Feature = Feature { + name: stringify!($feature), + get: get, + }; + &FEAT + } + )* + } + + impl Features { + fn status(&mut self, feature: &str) -> Option<(&mut bool, Status)> { + if feature.contains("_") { + return None + } + let feature = feature.replace("-", "_"); + $( + if feature == stringify!($feature) { + return Some((&mut self.$feature, stab!($stab))) + } + )* + None + } + } + ) +} + +macro_rules! stab { + (stable) => (Status::Stable); + (unstable) => (Status::Unstable); +} + +/// A listing of all features in Cargo +/// +/// "look here" +/// +/// This is the macro that lists all stable and unstable features in Cargo. +/// You'll want to add to this macro whenever you add a feature to Cargo, also +/// following the directions above. +/// +/// Note that all feature names here are valid Rust identifiers, but the `_` +/// character is translated to `-` when specified in the `cargo-features` +/// manifest entry in `Cargo.toml`. +features! { + pub struct Features { + + // A dummy feature that doesn't actually gate anything, but it's used in + // testing to ensure that we can enable stable features. + [stable] test_dummy_stable: bool, + + // A dummy feature that gates the usage of the `im-a-teapot` manifest + // entry. This is basically just intended for tests. + [unstable] test_dummy_unstable: bool, + } +} + +pub struct Feature { + name: &'static str, + get: fn(&Features) -> &bool, +} + +impl Features { + pub fn new(features: &[String], + warnings: &mut Vec) -> CargoResult { + let mut ret = Features::default(); + for feature in features { + ret.add(feature, warnings)?; + ret.activated.push(feature.to_string()); + } + Ok(ret) + } + + fn add(&mut self, feature: &str, warnings: &mut Vec) -> CargoResult<()> { + let (slot, status) = match self.status(feature) { + Some(p) => p, + None => bail!("unknown cargo feature `{}`", feature), + }; + + if *slot { + bail!("the cargo feature `{}` has already bene activated", feature); + } + + match status { + Status::Stable => { + let warning = format!("the cargo feature `{}` is now stable \ + and is no longer necessary to be listed \ + in the manifest", feature); + warnings.push(warning); + } + Status::Unstable if !nightly_features_allowed() => { + bail!("the cargo feature `{}` requires a nightly version of \ + Cargo, but this is the `{}` channel", + feature, + channel()) + } + Status::Unstable => {} + } + + *slot = true; + + Ok(()) + } + + pub fn activated(&self) -> &[String] { + &self.activated + } + + pub fn require(&self, feature: &Feature) -> CargoResult<()> { + if *(feature.get)(self) { + Ok(()) + } else { + let feature = feature.name.replace("_", "-"); + let mut msg = format!("feature `{}` is required", feature); + + if nightly_features_allowed() { + let s = format!("\n\nconsider adding `cargo-features = [\"{0}\"]` \ + to the manifest", feature); + msg.push_str(&s); + } else { + let s = format!("\n\n\ + this Cargo does not support nightly features, but if you\n\ + switch to nightly channel you can add\n\ + `cargo-features = [\"{}\"]` to enable this feature", + feature); + msg.push_str(&s); + } + bail!("{}", msg); + } + } +} + +fn channel() -> String { + env::var("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS").unwrap_or_else(|_| { + ::version().cfg_info.map(|c| c.release_channel) + .unwrap_or(String::from("dev")) + }) +} + +fn nightly_features_allowed() -> bool { + match &channel()[..] { + "nightly" | "dev" => true, + _ => false, + } +} diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 193a3e0a8..908899985 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -8,8 +8,9 @@ use serde::ser; use url::Url; use core::{Dependency, PackageId, Summary, SourceId, PackageIdSpec}; -use core::WorkspaceConfig; +use core::{WorkspaceConfig, Features, Feature}; use util::toml::TomlManifest; +use util::errors::*; pub enum EitherManifest { Real(Manifest), @@ -32,6 +33,8 @@ pub struct Manifest { patch: HashMap>, workspace: WorkspaceConfig, original: Rc, + features: Features, + im_a_teapot: Option, } /// When parsing `Cargo.toml`, some warnings should silenced @@ -239,6 +242,8 @@ impl Manifest { replace: Vec<(PackageIdSpec, Dependency)>, patch: HashMap>, workspace: WorkspaceConfig, + features: Features, + im_a_teapot: Option, original: Rc) -> Manifest { Manifest { summary: summary, @@ -253,7 +258,9 @@ impl Manifest { replace: replace, patch: patch, workspace: workspace, + features: features, original: original, + im_a_teapot: im_a_teapot, } } @@ -280,6 +287,10 @@ impl Manifest { &self.workspace } + pub fn features(&self) -> &Features { + &self.features + } + pub fn add_warning(&mut self, s: String) { self.warnings.push(DelayedWarning { message: s, is_critical: false }) } @@ -299,6 +310,17 @@ impl Manifest { ..self } } + + pub fn feature_gate(&self) -> CargoResult<()> { + if self.im_a_teapot.is_some() { + self.features.require(Feature::test_dummy_unstable()).chain_err(|| { + "the `im-a-teapot` manifest key is unstable and may not work \ + properly in England" + })?; + } + + Ok(()) + } } impl VirtualManifest { diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index a10dd39d8..2323b6c40 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -1,6 +1,7 @@ pub use self::dependency::Dependency; -pub use self::manifest::{Manifest, Target, TargetKind, Profile, LibKind, Profiles}; +pub use self::features::{Features, Feature}; pub use self::manifest::{EitherManifest, VirtualManifest}; +pub use self::manifest::{Manifest, Target, TargetKind, Profile, LibKind, Profiles}; pub use self::package::{Package, PackageSet}; pub use self::package_id::PackageId; pub use self::package_id_spec::PackageIdSpec; @@ -22,3 +23,4 @@ pub mod shell; pub mod registry; mod package_id_spec; mod workspace; +mod features; diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index 2ca6986e4..11f3e3457 100755 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -213,7 +213,6 @@ fn handle_cause(cargo_err: E, shell: &mut Shell) -> bool true } - pub fn version() -> VersionInfo { macro_rules! env_str { ($name:expr) => { env!($name).to_string() } diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 3bccb7c1f..72b083803 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -28,6 +28,10 @@ pub fn package(ws: &Workspace, opts: &PackageOpts) -> CargoResult> { let pkg = ws.current()?; let config = ws.config(); + if pkg.manifest().features().activated().len() > 0 { + bail!("cannot package or publish crates which activate nightly-only \ + cargo features") + } let mut src = PathSource::new(pkg.root(), pkg.package_id().source_id(), config); diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 242d5b207..7b802c317 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -14,7 +14,7 @@ use url::Url; use core::{SourceId, Profiles, PackageIdSpec, GitReference, WorkspaceConfig}; use core::{Summary, Manifest, Target, Dependency, PackageId}; -use core::{EitherManifest, VirtualManifest}; +use core::{EitherManifest, VirtualManifest, Features}; use core::dependency::{Kind, Platform}; use core::manifest::{LibKind, Profile, ManifestMetadata}; use sources::CRATES_IO; @@ -389,6 +389,10 @@ pub struct TomlProject { include: Option>, publish: Option, workspace: Option, + #[serde(rename = "cargo-features")] + cargo_features: Option>, + #[serde(rename = "im-a-teapot")] + im_a_teapot: Option, // package metadata description: Option, @@ -644,6 +648,9 @@ impl TomlManifest { }; let profiles = build_profiles(&me.profile); let publish = project.publish.unwrap_or(true); + let empty = Vec::new(); + let cargo_features = project.cargo_features.as_ref().unwrap_or(&empty); + let features = Features::new(&cargo_features, &mut warnings)?; let mut manifest = Manifest::new(summary, targets, exclude, @@ -655,6 +662,8 @@ impl TomlManifest { replace, patch, workspace_config, + features, + project.im_a_teapot, me.clone()); if project.license_file.is_some() && project.license.is_some() { manifest.add_warning("only one of `license` or \ @@ -667,6 +676,8 @@ impl TomlManifest { manifest.add_critical_warning(error); } + manifest.feature_gate()?; + Ok((manifest, nested_paths)) } diff --git a/tests/cargo-features.rs b/tests/cargo-features.rs new file mode 100644 index 000000000..2f88f3867 --- /dev/null +++ b/tests/cargo-features.rs @@ -0,0 +1,154 @@ +extern crate cargo; +extern crate cargotest; +extern crate hamcrest; + +use cargotest::ChannelChanger; +use cargotest::support::{project, execs}; +use hamcrest::assert_that; + +#[test] +fn feature_required() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + im-a-teapot = true + "#) + .file("src/lib.rs", ""); + assert_that(p.cargo_process("build") + .masquerade_as_nightly_cargo(), + execs().with_status(101) + .with_stderr("\ +error: failed to parse manifest at `[..]` + +Caused by: + the `im-a-teapot` manifest key is unstable and may not work properly in England + +Caused by: + feature `test-dummy-unstable` is required + +consider adding `cargo-features = [\"test-dummy-unstable\"]` to the manifest +")); + + assert_that(p.cargo("build"), + execs().with_status(101) + .with_stderr("\ +error: failed to parse manifest at `[..]` + +Caused by: + the `im-a-teapot` manifest key is unstable and may not work properly in England + +Caused by: + feature `test-dummy-unstable` is required + +this Cargo does not support nightly features, but if you +switch to nightly channel you can add +`cargo-features = [\"test-dummy-unstable\"]` to enable this feature +")); +} + +#[test] +fn unknown_feature() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + cargo-features = ["foo"] + "#) + .file("src/lib.rs", ""); + assert_that(p.cargo_process("build"), + execs().with_status(101) + .with_stderr("\ +error: failed to parse manifest at `[..]` + +Caused by: + unknown cargo feature `foo` +")); +} + +#[test] +fn stable_feature_warns() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + cargo-features = ["test-dummy-stable"] + "#) + .file("src/lib.rs", ""); + assert_that(p.cargo_process("build"), + execs().with_status(0) + .with_stderr("\ +warning: the cargo feature `test-dummy-stable` is now stable and is no longer \ +necessary to be listed in the manifest +[COMPILING] a [..] +[FINISHED] [..] +")); +} + +#[test] +fn nightly_feature_requires_nightly() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + im-a-teapot = true + cargo-features = ["test-dummy-unstable"] + "#) + .file("src/lib.rs", ""); + assert_that(p.cargo_process("build") + .masquerade_as_nightly_cargo(), + execs().with_status(0) + .with_stderr("\ +[COMPILING] a [..] +[FINISHED] [..] +")); + + assert_that(p.cargo("build"), + execs().with_status(101) + .with_stderr("\ +error: failed to parse manifest at `[..]` + +Caused by: + the cargo feature `test-dummy-unstable` requires a nightly version of Cargo, \ + but this is the `stable` channel +")); +} + +#[test] +fn cant_publish() { + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + im-a-teapot = true + cargo-features = ["test-dummy-unstable"] + "#) + .file("src/lib.rs", ""); + assert_that(p.cargo_process("build") + .masquerade_as_nightly_cargo(), + execs().with_status(0) + .with_stderr("\ +[COMPILING] a [..] +[FINISHED] [..] +")); + + assert_that(p.cargo("build"), + execs().with_status(101) + .with_stderr("\ +error: failed to parse manifest at `[..]` + +Caused by: + the cargo feature `test-dummy-unstable` requires a nightly version of Cargo, \ + but this is the `stable` channel +")); +} diff --git a/tests/cargotest/lib.rs b/tests/cargotest/lib.rs index 827502737..09f17b20e 100644 --- a/tests/cargotest/lib.rs +++ b/tests/cargotest/lib.rs @@ -44,6 +44,12 @@ fn _process(t: &OsStr) -> cargo::util::ProcessBuilder { .env("HOME", support::paths::home()) .env("CARGO_HOME", support::paths::home().join(".cargo")) .env("__CARGO_TEST_ROOT", support::paths::root()) + + // Force cargo to think it's on the stable channel for all tests, this + // should hopefully not suprise us as we add cargo features over time and + // cargo rides the trains + .env("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS", "stable") + .env_remove("__CARGO_DEFAULT_LIB_METADATA") .env_remove("RUSTC") .env_remove("RUSTDOC") @@ -65,6 +71,16 @@ fn _process(t: &OsStr) -> cargo::util::ProcessBuilder { return p } +pub trait ChannelChanger: Sized { + fn masquerade_as_nightly_cargo(&mut self) -> &mut Self; +} + +impl ChannelChanger for cargo::util::ProcessBuilder { + fn masquerade_as_nightly_cargo(&mut self) -> &mut Self { + self.env("__CARGO_TEST_CHANNEL_OVERRIDE_DO_NOT_USE_THIS", "nightly") + } +} + pub fn cargo_process() -> cargo::util::ProcessBuilder { process(&support::cargo_exe()) } -- 2.30.2